Skip to content

Bump helm chart version to 0.3.33, update app version to 1.17.33, ena…#36

Merged
willymwai merged 1 commit intomainfrom
chore/enable-self-service
Jun 25, 2025
Merged

Bump helm chart version to 0.3.33, update app version to 1.17.33, ena…#36
willymwai merged 1 commit intomainfrom
chore/enable-self-service

Conversation

@Pinchez25
Copy link
Contributor

@Pinchez25 Pinchez25 commented Jun 25, 2025

…ble self-service configuration, and add environment variable for self-service module.

What is the Purpose?

What was the approach?

How can the changes made get tested?

Are there any concerns to addressed further before or after merging this PR?

Mentions?

Issue(s) affected?

@Pinchez25
Bump helm chart version to 0.3.33, update app version to 1.17.33, ena…
c589a66 
…ble self-service configuration, and add environment variable for self-service module.
@willymwai
Copy link
Member

willymwai commented Jun 25, 2025

/describe

@willymwai
Copy link
Member

willymwai commented Jun 25, 2025

/improve

@willymwai
Copy link
Member

willymwai commented Jun 25, 2025

/review

1 similar comment
@willymwai
Copy link
Member

willymwai commented Jun 25, 2025

/review

@willymwai
Copy link
Member

willymwai commented Jun 25, 2025

/improve

@qodo-code-review
Copy link

qodo-code-review bot commented Jun 25, 2025

Title

Bump helm chart version to 0.3.33, update app version to 1.17.33, ena…

User description

…ble self-service configuration, and add environment variable for self-service module.

What is the Purpose?

What was the approach?

How can the changes made get tested?

Are there any concerns to addressed further before or after merging this PR?

Mentions?

Issue(s) affected?

PR Type

Enhancement

Description

  • Bump Helm chart version to 0.3.33

  • Update application version to 1.17.33

  • Enable self-service module configuration

  • Add environment variable for self-service module

Changes walkthrough 📝

Relevant files
Configuration changes
Chart.yaml
Bump chart and app versions                                                           

charts/jisort/Chart.yaml

  • Increment chart version from 0.3.32 to 0.3.33
  • Update appVersion from 1.17.32 to 1.17.33
    		• +2/-2     
    deployment.yaml
    Add self-service environment variable                                       

    charts/jisort/templates/deployment.yaml

  • Add FINERACT_MODULE_SELF_SERVICE_ENABLED environment variable
  • Set value from selfService.enabled configuration
    		• +2/-0     
    values.yaml
    Enable self-service configuration                                               

    charts/jisort/values.yaml

    • Enable self-service module by setting enabled to true
    		 +1/-0     
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    		•

    @qodo-code-review
    Copy link

    qodo-code-review bot commented Jun 25, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Configuration Validation

    The new environment variable FINERACT_MODULE_SELF_SERVICE_ENABLED is added but there's no validation that the selfService.enabled value exists in values.yaml or has a default fallback if undefined.

     
    - name: FINERACT_MODULE_SELF_SERVICE_ENABLED
  value: {{ .Values.selfService.enabled | quote }}
    Default Value

    The selfService.enabled is set to true by default, which enables the self-service module for all deployments. Consider if this should be false by default for security or compatibility reasons.

     
    enabled: true
resources:

    1 similar comment
    @qodo-code-review
    Copy link

    qodo-code-review bot commented Jun 25, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Configuration Validation

    The new environment variable FINERACT_MODULE_SELF_SERVICE_ENABLED is added but there's no validation that the selfService.enabled value exists in values.yaml or has a default fallback if undefined.

     
    - name: FINERACT_MODULE_SELF_SERVICE_ENABLED
  value: {{ .Values.selfService.enabled | quote }}
    Default Value

    The selfService.enabled is set to true by default, which enables the self-service module for all deployments. Consider if this should be false by default for security or compatibility reasons.

     
    enabled: true
resources:

    @qodo-code-review
    Copy link

    qodo-code-review bot commented Jun 25, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Latest suggestions up to c589a66

    CategorySuggestion                                                                                                                                    Impact
    General
    Add toString filter for boolean conversion

    The boolean value should be converted to string before quoting to ensure proper
    YAML rendering. Using toString filter prevents potential issues with
    boolean-to-string conversion in Helm templates.

    charts/jisort/templates/deployment.yaml [131-132]

     - name: FINERACT_MODULE_SELF_SERVICE_ENABLED
-  value: {{ .Values.selfService.enabled | quote }}
+  value: {{ .Values.selfService.enabled | toString | quote }}
    • Apply / Chat
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion correctly applies a Helm best practice. While | quote alone would likely work for the boolean value true, explicitly converting it with | toString first makes the template more robust and readable, preventing potential issues with type coercion.

    		Low
    • More

    Previous suggestions

    Suggestions up to commit c589a66
    CategorySuggestion                                                                                                                                    Impact
    General
    Add toString filter for boolean conversion

    The boolean value should be converted to string before quoting to ensure proper
    YAML rendering. Using toString filter prevents potential issues with
    boolean-to-string conversion in Helm templates.

    charts/jisort/templates/deployment.yaml [131-132]

     - name: FINERACT_MODULE_SELF_SERVICE_ENABLED
-  value: {{ .Values.selfService.enabled | quote }}
+  value: {{ .Values.selfService.enabled | toString | quote }}
    Suggestion importance[1-10]: 5

    __

    Why: The suggestion is correct. While | quote on a boolean value often works as intended in Helm, explicitly converting the value to a string using | toString before quoting is a best practice. This ensures robustness and prevents potential issues with how the value is interpreted.

    		Low

    @willymwai willymwai merged commit e2056c3 into main Jun 25, 2025
    1 check passed
    @willymwai willymwai deleted the chore/enable-self-service branch June 25, 2025 11:19
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @willymwai willymwai willymwai approved these changes

    Assignees

    No one assigned

    Labels

    Review effort 2/5

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    @Pinchez25 @willymwai